Skip to content

fix: Handling of sync and async requests#313

Draft
casperlehmann wants to merge 21 commits intotekumara:mainfrom
casperlehmann:clehmann/fix-handling-of-sync-and-async
Draft

fix: Handling of sync and async requests#313
casperlehmann wants to merge 21 commits intotekumara:mainfrom
casperlehmann:clehmann/fix-handling-of-sync-and-async

Conversation

@casperlehmann
Copy link
Copy Markdown
Contributor

@casperlehmann casperlehmann commented Mar 2, 2026

Summary

This PR fixes Fakesnow's compatibility with both Node.js and Python Snowflake SDKs by implementing the correct response format and query result polling behavior.

Changes

1. Add status code for successful queries (ff43a93)

  • Added code: "0" field to POST response to explicitly signal "results ready"
  • This field must be set in the response, since the clients are not doing null handling:
    • Without this field, SDKs interpreted responses differently:
      • Node.js SDK: Assumed async query → attempted GET request (decision logic)
      • Python SDK: Treated as request failure → retried with new POST requests (polling logic)
  • Now both SDKs correctly use inline results without additional requests.
  • Returning code: "0" does mean that we are essentially never serving async responses. But since I'm still not sure how to make that decision programmatically, I have left it like this for now.

2. Add required response fields (a711a9c)

  • Added fields expected by both SDKs: queryId, sqlText, returned, total, etc.
  • Ensures response structure matches SDK expectations for both Node.js (rowset) and Python (rowsetBase64)

3. Add async GET endpoint support (13ad8de)

  • Implemented get_cached_query_result() function to retrieve cached query results
  • Mounted /queries/{query_id}/result endpoint for async polling behavior
  • Allows SDKs to fetch results via GET when code: "333333" (in progress) or code: "333334" (async execution) is returned
  • Added results cache (conn.results_cache) with LRU eviction (max 50 entries) to store query results by queryId
  • Note: This polling endpoint was hit on every single query sent by the async Node.js SDK. After adding the "code" field in the query_request response, this is no longer the case. Since we added a default value (code: "0" -- completed) to query_request, the request will never enter the polling state, and this GET endpoint will no longer get hit. But I am leaving it in, since it is already working and, ideally, the query_request should be able to return in-progress values for code as well (perhaps by setting a timeout in on the query execution).

4. Add SafeJSONResponse for datetime serialization (88e0bfd)

  • Created custom response class to handle non-JSON-serializable types like datetime
  • Prevents serialization errors when query results contain temporal data
  • Uses custom JSON encoder that converts problematic types to strings

5. Store results cache in tuple format with rowtype (a16fd35, 63e73fe)

  • Internal cache only: Results cache (conn.results_cache) is never exposed externally - only used for result_scan() and get_results_from_sfqid()
  • Changed cache format from dictionary to 6-tuple: (arrow_table, rowcount, last_sql, last_params, last_transformed, rowtype)
  • Rowtype is required for GET /queries/{query_id}/result endpoint to generate rowsetBase64 (Arrow IPC format needed by Python SDK)
  • Cache uses OrderedDict for LRU eviction (max 50 entries) to prevent unbounded memory growth
  • Fixed tuple format consistency: both server.py and cursor.py now store 6-tuple with rowtype

6. Add support for persisting database in server mode

Allow clients the option to set FAKESNOW_DB_PATH when running in server mode, in order to persist data on disk.

Technical Details

The changes implement the private SDK API (/queries/v1/query-request) used by Node.js and Python Snowflake connectors, not the public Snowflake SQL API. Key differences:

  • Success code: "0" (not "090001" from public API)
  • Data fields: rowset/rowsetBase64 (not data array from public API)
  • Async codes: "333333" (in progress), "333334" (detached) - defined in Node.js SDK and Python SDK

See also: Snowflake SQL API Reference for comparison with public API behavior.

@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from ed10609 to 2b6a6ae Compare March 3, 2026 12:46
@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from c9f38c4 to a57e6ad Compare March 3, 2026 13:43
casperlehmann and others added 3 commits March 3, 2026 14:48
Co-Authored-By: Andrew Bird <25768973+abirdzendesk@users.noreply.github.com>
@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from a57e6ad to ff43a93 Compare March 3, 2026 13:51
@casperlehmann casperlehmann changed the title Clehmann/fix handling of sync and async Fix handling of sync and async requests Mar 3, 2026
@casperlehmann casperlehmann changed the title Fix handling of sync and async requests Fix: Handling of sync and async requests Mar 3, 2026
@casperlehmann casperlehmann changed the title Fix: Handling of sync and async requests fix: Handling of sync and async requests Mar 3, 2026
@casperlehmann casperlehmann marked this pull request as ready for review March 3, 2026 15:28
@casperlehmann casperlehmann marked this pull request as draft March 3, 2026 15:36
casperlehmann and others added 6 commits March 4, 2026 15:28
The results cache was storing a dictionary format, but result_scan() and
get_results_from_sfqid() expect a tuple of (arrow_table, rowcount, last_sql,
last_params, last_transformed). Updated server.py to store the tuple format
and reconstruct the response dictionary when needed for GET requests.

Fixes test_server_async_query_with_retrieval.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
test: add comprehensive tests for results cache format

Add 6 new tests validating:
- result_scan() end-to-end functionality
- GET /queries/{query_id}/result endpoint
- LRU cache eviction (>50 entries)
- Empty result set caching
- Multiple query cache isolation
- Error handling for missing queries

Also fixes discovered bugs:
- Add rowtype as 6th element in cache tuple (was missing)
- Change results_cache from dict to OrderedDict for LRU eviction

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The results cache now stores a consistent 6-tuple format across both
server.py and cursor.py. Previously, server.py stored 6-tuple with rowtype
but cursor.py stored 5-tuple without it, causing "not enough values to
unpack" errors when result_scan() or get_results_from_sfqid() tried to
restore cached results.

Rowtype computation skips DESCRIBE queries to avoid infinite recursion.

Fixes test_execute_async_and_get_results_from_sfqid.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from a16fd35 to 8aa69af Compare March 4, 2026 14:28
The server now respects the FAKESNOW_DB_PATH environment variable if set, allowing users to specify a persistent database
  file path. If not set, it falls back to the default in-memory database behavior. This enables multiple connections to share either
   a persistent or in-memory database depending on configuration needs.
@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from af502dd to 49b74aa Compare March 5, 2026 12:55
@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from 40d6e39 to e349548 Compare March 5, 2026 13:20
@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from e349548 to ee1c542 Compare March 5, 2026 13:21
@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from e0a0ac1 to cec6926 Compare March 5, 2026 14:18
@casperlehmann casperlehmann force-pushed the clehmann/fix-handling-of-sync-and-async branch from 4f1534a to c568aa2 Compare March 5, 2026 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant